Skip to content

Remove intX and floatX from distributions#7114

Merged
ricardoV94 merged 18 commits intopymc-devs:mainfrom
aerubanov:remove-floatx-calls-from-distribution
Feb 17, 2024
Merged

Remove intX and floatX from distributions#7114
ricardoV94 merged 18 commits intopymc-devs:mainfrom
aerubanov:remove-floatx-calls-from-distribution

Conversation

@aerubanov
Copy link
Contributor

@aerubanov aerubanov commented Jan 23, 2024 •
edited by ricardoV94
Loading

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7114.org.readthedocs.build/en/7114/

@aerubanov aerubanov marked this pull request as draft January 23, 2024 15:02
@codecov
Copy link

codecov bot commented Jan 23, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (627a8dd) 90.41% compared to head (00ca006) 92.35%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7114      +/-   ##
==========================================
+ Coverage   90.41%   92.35%   +1.93%     
==========================================
  Files         101      101              
  Lines       16930    16922       -8     
==========================================
+ Hits        15308    15629     +321     
+ Misses       1622     1293     -329
Files Coverage Δ
pymc/distributions/bound.py 100.00% <100.00%> (ø)
pymc/distributions/continuous.py 97.53% <100.00%> (-0.01%) ⬇️
pymc/distributions/discrete.py 99.10% <100.00%> (-0.01%) ⬇️
pymc/distributions/mixture.py 94.95% <100.00%> (-0.03%) ⬇️
pymc/distributions/multivariate.py 93.78% <100.00%> (-0.02%) ⬇️
pymc/distributions/simulator.py 84.17% <100.00%> (-0.12%) ⬇️
pymc/distributions/timeseries.py 94.45% <100.00%> (ø)
pymc/distributions/transforms.py 98.47% <100.00%> (-0.02%) ⬇️

... and 6 files with indirect coverage changes

aerubanov
aerubanov commented Jan 29, 2024
View reviewed changes
@aerubanov
Copy link
Contributor Author

aerubanov commented Jan 29, 2024 •
edited
Loading

Also, tests/variational/test_inference.py::test_fit_start[SVGD-mini] fails but I do not sure is it related with changes in this PR. This test just skipped when using pytest==7.* and fails with pytest==8.0. I think it`s related with exceptions and warning handling order inside pytest.

@aerubanov aerubanov marked this pull request as ready for review January 29, 2024 14:58
@aerubanov aerubanov changed the title remove floatX() from distribution/continuous.py remove floatX() from distributions Jan 29, 2024
@aerubanov
Copy link
Contributor Author

aerubanov commented Jan 29, 2024

@ricardoV94 could you please take a look?

@ricardoV94 ricardoV94 changed the title remove floatX() from distributions Remove floatX() from distributions Feb 5, 2024
@ricardoV94 ricardoV94 changed the title Remove floatX() from distributions Remove floatX from distributions Feb 5, 2024
This was referenced Feb 5, 2024
Closed
Closed
ricardoV94
ricardoV94 reviewed Feb 6, 2024
View reviewed changes
tests/distributions/test_truncated.py Outdated
Copy link
Member

@ricardoV94 ricardoV94 Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? We shouldn't have changed results numerically

ricardoV94
ricardoV94 reviewed Feb 6, 2024
View reviewed changes
ricardoV94
ricardoV94 reviewed Feb 6, 2024
View reviewed changes
ricardoV94
ricardoV94 reviewed Feb 6, 2024
View reviewed changes
@ricardoV94 ricardoV94 mentioned this pull request Feb 6, 2024
Closed
@ricardoV94 ricardoV94 changed the title Remove floatX from distributions Remove intX and floatX from distributions Feb 6, 2024
This was referenced Feb 6, 2024
Closed
Closed
aerubanov and others added 8 commits February 9, 2024 15:34
@aerubanov
fix test in TestInitvalAssignment
58635f3
@ricardoV94 @aerubanov
Fix check in CAR make_node to also work with SparseConstants
f564f46
@aerubanov @ricardoV94
Update pymc/distributions/multivariate.py
9e77a0f 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@aerubanov @ricardoV94
Update tests/distributions/test_mixture.py
7837795 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@aerubanov @ricardoV94
Update pymc/distributions/multivariate.py
f2b5ef5 
Co-authored-by: Ricardo Vieira <28983449+ricardoV94@users.noreply.github.com>
@aerubanov
add .astype(floatX)
bab56c1
@aerubanov
remove rtol from test
8d26013
@aerubanov
fix tests
00ca006
@aerubanov
Copy link
Contributor Author

aerubanov commented Feb 9, 2024

@ricardoV94 I added some changes, could you please take a look?

@aerubanov
Copy link
Contributor Author

aerubanov commented Feb 16, 2024

@ricardoV94 just friendly reminder about PR)

@ricardoV94 ricardoV94 merged commit 3a304d6 into pymc-devs:main Feb 17, 2024
@ricardoV94
Copy link
Member

ricardoV94 commented Feb 17, 2024

Thanks @aerubanov !

@aerubanov aerubanov deleted the remove-floatx-calls-from-distribution branch February 19, 2024 08:02
This was referenced Feb 23, 2024
Closed
Closed
@ricardoV94 ricardoV94 added the major Include in major changes release notes section label Mar 15, 2024
This was referenced May 22, 2024
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ricardoV94 ricardoV94 ricardoV94 approved these changes

Assignees

No one assigned

Labels

maintenance major Include in major changes release notes section

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Get rid of floatX calls inside distributions Silent integer overflow in test_values Int32 or Int64 for discrete?

2 participants

@aerubanov @ricardoV94